Status filter checkboxes #310
Conversation
| type StatusInfo = { | ||
| type: string; | ||
| status: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Converts a status string into an info object | ||
| */ | ||
| export const statusToType = (statusString: string): StatusInfo => { | ||
| const [status, type] = statusString.split(' '); | ||
| return { status, type }; | ||
| }; |
There was a problem hiding this comment.
Is it necessary to include a statusToType sort of function in the filters service?
The filters service should be exclusively kept for storing and updating filters in line with the SRP.
I think this characterization of information from certain filters should be done where the filters are applied i.e., in dropdownfilter.ts in this case.
I would suggest simply using the status string in dropdownfilter.ts and parsing it appropriately as part of filtering
| export const DEFAULT_FILTER: Filter = { | ||
| title: '', | ||
| status: 'all', | ||
| status: ['open pullrequest', 'merged pullrequest', 'closed pullrequest', 'open issue', 'closed issue'], |
There was a problem hiding this comment.
As per the description of #296, closed pullrequest should not be selected by default
| export const statusToType = (statusString: string): StatusInfo => { | ||
| const [status, type] = statusString.split(' '); | ||
| return { status, type }; | ||
| }; |
There was a problem hiding this comment.
Is the export necessary?
There was a problem hiding this comment.
Would it make more sense to name the function something similar to infoFromStatus since we are returning a type StatusInfo object and not just a Type which the current name seems to imply
The label sanitation ensures that keep filters work with labels. The method is updated to work with deselected labels as well.
Summary:
Fixes #296
Type of change:
Changes Made:
Filter::statustype fromstringtostring[]dropdownfilterto filter according to the new representationFilter::typeis correctScreenshots:
Proposed Commit Message:
Checklist: